Skip to content

ext/zip: addGlob() and addPattern() ignore their default options. - #23108

Closed
devnexen wants to merge 2 commits into
php:PHP-8.4from
devnexen:ext_zip_add_glob_pattern
Closed

ext/zip: addGlob() and addPattern() ignore their default options.#23108
devnexen wants to merge 2 commits into
php:PHP-8.4from
devnexen:ext_zip_add_glob_pattern

Conversation

@devnexen

@devnexen devnexen commented Aug 7, 2026

Copy link
Copy Markdown
Member

php_zip_parse_options() held the defaults but only ran for a non-empty options array, so opts stayed zeroed otherwise: comp_method 0 is CM_STORE and flags 0 drops FL_OVERWRITE. Entries were therefore stored uncompressed, and an already present entry name failed the call instead of being replaced. The defaults now live in PHP_ZIP_DEFAULT_OPTIONS, applied at declaration.

php_zip_parse_options() held the defaults but only ran for a non-empty
options array, so opts stayed zeroed otherwise: comp_method 0 is CM_STORE
and flags 0 drops FL_OVERWRITE. Entries were therefore stored uncompressed,
and an already present entry name failed the call instead of being replaced.
The defaults now live in PHP_ZIP_DEFAULT_OPTIONS, applied at declaration.
@devnexen
devnexen marked this pull request as ready for review August 7, 2026 19:40
@devnexen
devnexen requested a review from LamentXU123 as a code owner August 7, 2026 19:40

@LamentXU123 LamentXU123 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could we add the same default-options regression coverage for ZipArchive::addPattern()?

Both entry points share php_zip_add_from_pattern(), only addGlob() was covered.

@LamentXU123 LamentXU123 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good to me!

@devnexen devnexen closed this in 8d5b0d6 Aug 8, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants